refactor: improve cli #3
Conversation
feat: Add new command. test: Update tests
aarcex3
changed the title
There was a problem hiding this comment.
Hey! Thanks for the pull request, your work is really appreciated. I totally agree with having an option to generate individual resources. In addition to some of the comments that I've left, I also think that you should not use RenderingContext for rendering a resource. Instead create another context class (something like ResourceRenderingContext with resource specific context fields) and use it to render resources instead.
.vscode/settings.json
Outdated
There was a problem hiding this comment.
Remove IDE configuration files from the commit.
| litestar project init --app-name MyProject | ||
| ``` | ||
|
|
||
| This command is used to initialize a Litestar project named MyProject in the current working directory. MyProject will have the following tree structure: | ||
|
|
||
| ``` | ||
| ```bash |
There was a problem hiding this comment.
Is there a specific reason behind this change?
There was a problem hiding this comment.
It looks cleaner in my opinion, I can remove it if you want
pyproject.toml
Outdated
| @@ -306,6 +306,7 @@ distribution = true | |||
| [tool.pdm.dev-dependencies] | |||
| dev = [ | |||
| "litestar[standard,cryptography,jinja]>=2.9.1", | |||
| "black>=24.8.0", | |||
There was a problem hiding this comment.
The project uses ruff for linting. Remove black dependency and lint the PR using ruff.
src/litestar_manage/cli.py
Outdated
| @click.group(cls=LitestarGroup) | ||
| def cli(): | ||
| pass |
There was a problem hiding this comment.
This is meant to be an extension of the LItestar's CLI, so it doesn't really make sense for the group to be named CLI. Rename this group back to project.
| render_template(template_dir, output_dir, ctx, run_ruff=True) | ||
|
|
||
| packages_to_install = ["litestar"] | ||
| venv_name = "venv" |
There was a problem hiding this comment.
We don't want to make assumptions about what the virtual environment name is, nor do we want to hard code that assumption. Factor out the virtual environment name into a constant and possibly make a list of possible venv names to check against.
Would it be ok to make a subclass of RenderingContext for this matter? Or maybe create an abstract RenderingContext from which the other can inherent? Because in most cases we just render jinja files (and others), right? |
|
Since |
|
@aarcex3 Thank you for your efforts. |
Code proposal
The main cli has been refactor and a new command has been added, namely
generate.This is a proposal to generate a folder like the ones on the fullstack example, where a resource consists of a controller, service, dtos, models and a repository file.
The proposal is to follow a Nestjs like philosophy when it comes to the cli.
Related Issue
None